Bound test running on dune:version < 3.24 for packages with brittle path handling of dune variables - #30157

This PR only add upper bounds on dune to tests, and all the failures (of which there are many) only speak to underlying problems with the affected packages in the chosen solution, and do not (afaict) reflect any issues with these bounds.

thanks

#30167 (comment) has some revdeps failures which could be related. For example, that PR has a failure for ppx_minidebug.2.3.0 but this PR only adapts older versions. Is this PR somehow incomplete?

@sim642, This PR is merged, so complete in that sense! But I was only able to make fixes for the packages that turned up in our CI results, and it's possible that either those revdeps didn't run, or they ran but failed for unrelated CI internal problems, or also that they somehow slipped thru the cracks.

I attempted to do preemptive fixes for everything we detected at the time, but I am not surprised that some packages with brittle tests have slipped thru, and will probably take some coordinated and sustained effort to get all of these fixed in a (possibly long) tail of remaining brittle tests.

[...] that they somehow slipped thru the cracks.

I think this might be the case because opam-ci for #30092 is https://opam.ci.ocaml.org/github/ocaml/opam-repository/commit/662e6a3427c777b88986bc9d015759eaaab0ea96. That massive list of failures also includes, e.g., ppx_minidebug.2.3.0.

My perspective and question are for the opam-repository maintainer team about who should fix such issues? Is it the maintenance team? The dune maintainers? Or the maintainer(s) of an unrelated package that fails?

I'm not sure if this is even relevant, from my perspective: we all try to keep the opam-repository in a nice state, which usually is the case (thanks to the maintainers, thanks to the CI systems, ...). At the same time there's some suffering e.g. of the CI output, some suffering from temporary CI failures, and some suffering from the CI not being able to write comments.

Related is that there seems to be not sufficient funding to make the opam-repository always work for everyone in all potential configurations. But I also guess achieving this is very very hard - so let's aim for this, but let's as well not be disappointed if we don't achieve it at every commit.

So, after saying that, I'd be mostly interested what the main active opam-repository maintainers (but as well package authors) would think as the most valuable improvement (may it be process (I know Anil is in favour of merge queues), or automated systems (CI, whatnot)).

I personally have a 80% finished "opam-review", which checks for common mistakes and whether the submission is aligned with the policy. This software does not attempt to compile anything, and outputs a Markdown file which can be put as GitHub comment. Would such a thing be useful? But I'm sure other ideas are around :)

@sim642,

[...] that they somehow slipped thru the cracks.

I think this might be the case because opam-ci for #30092 is https://opam.ci.ocaml.org/github/ocaml/opam-repository/commit/662e6a3427c777b88986bc9d015759eaaab0ea96. That massive list of failures also includes, e.g., ppx_minidebug.2.3.0.

Yes, you're right! I'll review the triage process to try to understand why versions weren't identified. In theory, they should have been, but I'm working on a fair bit of text munging scripts to deal with the volume here, and then a best effort to submit fixes as needed, and I'm not surprised I missed some packages. I regret the error however and appreciate you point it out.

@hannesm

My perspective and question are for the opam-repository maintainer team about who should fix such issues? Is it the maintenance team? The dune maintainers? Or the maintainer(s) of an unrelated package that fails?

AFAIU, when the scope of "such issues" is changes to bounds or filters on existing package versions to deal with breaking changes, then the current responsibility is shared between opam repo maintainers and package authors publishing the breaking change. But ideally the package will not be merged until any needed bounds/filters are added. (In the case, the sheer volume of noise we have to cope with due to broken packages or CI problems just means I have failed to reach that ideal). Exactly the dynamics you diagnose.

what the main active opam-repository maintainers (but as well package authors) would think as the most valuable improvement

Getting more people actively working on maintaining and sustaining the repo is one obviously important improvement. We also have a huge back log of suggestions for process and systems to improve things, as I'm sure you are aware, since you have both contributed to suggestions and process :)

I personally have a 80% finished "opam-review", which checks for common mistakes and whether the submission is aligned with the policy. This software does not attempt to compile anything, and outputs a Markdown file which can be put as GitHub comment. Would such a thing be useful? But I'm sure other ideas are around :)

This sounds useful, for sure! I wonder how much overlap there is in this with the existing CI lint tool and/how how many of the checks you have implemented could be ported to that existing linter. It would be useful to see.

I'd note that, IMO, this closed PR is probably not the best venue for proposing policy, process, or system improvements :D

Ah, this is why ppx_minidebug was not fixed in this PR: it's tests have been failing since at least dune 3.21 (as per our revdep failure logs from https://github.com/ocaml/dune/wiki/Reverse-dependencies-CI-logs ). I just tried to install it with-test using dune 3.21.1 and the tests are failing with the same CI reported result (The compilation of ppx_minidebug.2.3.0 failed at "dune build -p ppx_minidebug -j 11 @install @runtest".) and error which is just a result of brittle test fixtures (expect tests of this sort are a scourge on our CI system):

[ERROR] The compilation of ppx_minidebug.2.3.0 failed at "dune build -p ppx_minidebug -j 11 @install @runtest".

#=== ERROR while compiling ppx_minidebug.2.3.0 ================================#
# context     2.5.1 | macos/x86_64 | ocaml-base-compiler.5.4.0 | https://opam.ocaml.org#89a06fb4a7950d434fafca1593197d35ae530eb2
# path        ~/.opam/5.4.0/.opam-switch/build/ppx_minidebug.2.3.0
# command     ~/.opam/opam-init/hooks/sandbox.sh build dune build -p ppx_minidebug -j 11 @install @runtest
# exit-code   1
# env-file    ~/.opam/log/ppx_minidebug-63220-17c0d2.env
# output-file ~/.opam/log/ppx_minidebug-63220-17c0d2.out
### output ###
# +    YYYY-MM-DD HH:MM:SS.NNNNNN -04:00 - baz end
# [...]
#      |}]
#
# @@ -99,7 +99,7 @@ let%expect_test "%debug_show flushing to stdout" =
#    print_endline output;
#    [%expect
#      {|
# -    BEGIN DEBUG SESSION at elapsed NNN.NNxs / NNNNNNNns, corresponding to time YYYY-MM-DD HH:MM:SS.NNNNNN
# +    BEGIN DEBUG SESSION at elapsed NNN.NNxs / NNNNNNNns, corresponding to time YYYY-MM-DD HH:MM:SS.NNNNNN -04:00
#      bar begin "test/test_expect_test.ml":77:21: NNN.NNxs / NNNNNNNns
#       x = { Test_expect_test.first = 7; second = 42 }
#       y begin "test/test_expect_test.ml":78:8: NNN.NNxs / NNNNNNNns

IIUC, it just happens that the new failure you are seeing in the linked revdeps are failing (also due to brittle test fixtures) but earlier in the test reporting process. In this case, adding a filter on dune would prevent the error you are seeing, but not fix the underlying issue of failing test in this package due to the brittle result. I'll propose a change to remove the with-test command all together here tho.

When we are triaging failures for dune release, we take the diff of new failure that were not reported on the last known good version, which is why this was ignored.